[lockfile-explorer] Isolate .pnpmcfile.cjs execution and add syntax highlighter by octogonz · Pull Request #5366 · microsoft/rushstack

octogonz · 2025-09-17T18:05:37Z

Summary

More progress updating Lockfile Explorer.

Details

-.pnpmcfile.cjs is now evaluated in an isolated thread; the earlier approach of simply calling require() on it would cause memory leaks and incorrect caching

The file dump tabs now use a syntax highlighter

How it was tested

Tested manually

Impacted documentation

None.

@iclanton @nickpape @william2958 @dmichon-msft

…alhost

dmichon-msft · 2025-09-17T19:40:48Z

apps/lockfile-explorer/src/graph/PnpmfileRunner.ts

+    });
+  }
+
+  public async disposeAsync(): Promise<void> {


You should use [Symbol.asyncDispose] so that this object can be used with await using

Good suggestion. Fixed.

I had to upgrade Prettier to support this syntax

Turns out that await using is not supported by Node 18 or Jest. I will revert those changes.

@iclanton @dmichon-msft

dmichon-msft · 2025-09-17T19:41:16Z

apps/lockfile-explorer/src/graph/pnpmfileRunnerWorkerThread.ts

+  pnpmfileModuleError = error;
+}
+
+parentPort?.on('message', async (message: IRequestMessage) => {


parentPort not existing should be an immediate fatal error.

dmichon-msft · 2025-09-17T19:41:32Z

apps/lockfile-explorer/src/graph/pnpmfileRunnerWorkerThread.ts

+let pnpmfileModuleError: Error | undefined = undefined;
+
+try {
+  if (fs.existsSync(resolvedPath)) {


This is redundant. You're already catching the error, and you should skip the worker altogether if you know the file doesn't exist to save on all the messaging overhead.

dmichon-msft · 2025-09-17T19:42:57Z

apps/lockfile-explorer/src/graph/pnpmfileRunnerWorkerThread.ts

+  }
+
+  try {
+    if (!pnpmfileModule || !pnpmfileModule.hooks || typeof pnpmfileModule.hooks.readPackage !== 'function') {


I'd suggest instead sending a message on initialization to the host telling it which hooks are present, then don't bother sending messages when the readPackage hook isn't defined.

Combine with an initializeAsync method on PnpmfileRunner that you wait for before calling the individual APIs.

It's a good idea, but in the interest of time I need to keep the implementation simple for now.

dmichon-msft · 2025-09-17T19:43:47Z

apps/lockfile-explorer/src/graph/pnpmfileRunnerWorkerThread.ts

+}
+
+parentPort?.on('message', async (message: IRequestMessage) => {
+  const { id, packageJson } = message;


I find it generally useful to have false be one of the valid message values, and have that be a signal to gracefully shut down the worker. See, e.g. the ModuleMinifierWorker.

This is a good suggestion but I would need to add some safeguards to guarantee the child really does terminate. I'll do it in a future PR.

dmichon-msft · 2025-09-17T19:46:34Z

apps/lockfile-explorer-web/src/containers/PackageJsonViewer/index.tsx

+      const packageJSONFile: IPackageJson | undefined = await readPackageJsonAsync(packageName);
      setPackageJSON(packageJSONFile);
-      const parsedJSON = await readPackageSpecAsync(packageName);
+      const parsedJSON: IPackageJson | undefined = await readPackageSpecAsync(packageName);
      setParsedPackageJSON(parsedJSON);


Parallelize these with Promise.all?

Probably premature optimization at this point.

apps/lockfile-explorer-web/src/containers/PackageJsonViewer/CodeBox.tsx

iclanton · 2025-09-18T19:31:57Z

apps/lockfile-explorer-web/src/containers/PackageJsonViewer/CodeBox.tsx

+export type PrismLanguage =
+  | 'plain'
+  | 'plaintext'
+  | 'text'
+  | 'txt'
+  | 'markup'
+  | 'html'
+  | 'mathml'
+  | 'svg'
+  | 'xml'
+  | 'ssml'
+  | 'atom'
+  | 'rss'
+  | 'regex'
+  | 'clike'
+  | 'javascript'
+  | 'js'
+  | 'actionscript'
+  | 'coffeescript'
+  | 'coffee'
+  | 'javadoclike'
+  | 'css'
+  | 'yaml'
+  | 'yml'
+  | 'markdown'
+  | 'md'
+  | 'graphql'
+  | 'sql'
+  | 'typescript'
+  | 'ts'
+  | 'jsdoc'
+  | 'flow'
+  | 'n4js'
+  | 'n4jsd'
+  | 'jsx'
+  | 'tsx'
+  | 'swift'
+  | 'kotlin'
+  | 'kt'
+  | 'kts'
+  | 'c'
+  | 'objectivec'
+  | 'objc'
+  | 'reason'
+  | 'rust'
+  | 'go'
+  | 'cpp'
+  | 'python'
+  | 'py'
+  | 'json'
+  | 'webmanifest';


These strings are not expressed in the Prism .d.ts file. They are apparently only available at runtime.

iclanton · 2025-09-18T19:34:33Z

apps/lockfile-explorer-web/webpack.config.js

        // "The following entrypoint(s) combined asset size exceeds the recommended limit."
-        // maxEntrypointSize: 500000,
-        // maxAssetSize: 500000
+        maxEntrypointSize: Infinity,


If you're setting these to infinity, why not just turn off performance?

@iclanton Is there a better way to do that? I tried undefined but in the merge it doesn't undo the rig's setting.

It's performance: false

iclanton · 2025-09-18T19:37:07Z

apps/lockfile-explorer/src/cli/explorer/ExplorerCommandLineParser.ts

+        const pnpmfilePath: string = path.join(
+          appState.lfxWorkspace.workspaceRootFullPath,
+          appState.lfxWorkspace.pnpmLockfileFolder,
+          '.pnpmfile.cjs'
+        );


Suggested change

const pnpmfilePath: string = path.join(

appState.lfxWorkspace.workspaceRootFullPath,

appState.lfxWorkspace.pnpmLockfileFolder,

'.pnpmfile.cjs'

);

const { lfxWorkspace: { workspaceRootFullPath, pnpmLockfileFolder } } = appState;

const pnpmfilePath: string = `${workspaceRootFullPath}/${pnpmLockfileFolder}/.pnpmfile.cjs`;

This would incorrectly create double-slashes when pnpmLockfileFolder is the empty string

apps/lockfile-explorer/src/cli/explorer/ExplorerCommandLineParser.ts

apps/lockfile-explorer/src/graph/PnpmfileRunner.ts

iclanton · 2025-09-18T19:41:08Z

apps/lockfile-explorer/src/graph/pnpmfileRunnerWorkerThread.ts

+// debugger;
+
+const { pnpmfilePath } = workerData;
+const resolvedPath: string = path.resolve(pnpmfilePath);


Is this ever not absolute?

It is absolute, but only because in the current implementation, an absolute path is used by the caller of the class constructor in the other thread. It is a very brittle assumption.

apps/lockfile-explorer/src/graph/test/PnpmfileRunner.test.ts

…the generated temp file

octogonz added 4 commits September 17, 2025 14:01

Enable syntax highlighting for pnpmfile.cjs tab

775f982

Highlight package.json as well

e03c5b0

Introduce PnpmfileRunner.ts to isolate .pnpmfile.cjs execution

717043a

rush change

0f8393f

octogonz requested review from D4N14L, apostolisms, dmichon-msft, iclanton and patmill as code owners September 17, 2025 18:05

github-project-automation bot added this to Bug Triage Sep 17, 2025

github-project-automation bot moved this to Needs triage in Bug Triage Sep 17, 2025

Disable webpack bundle size limits, since this app is served from loc…

0342885

…alhost

dmichon-msft reviewed Sep 17, 2025

View reviewed changes

iclanton reviewed Sep 18, 2025

View reviewed changes

apps/lockfile-explorer-web/src/containers/PackageJsonViewer/CodeBox.tsx Outdated Show resolved Hide resolved

iclanton approved these changes Sep 18, 2025

View reviewed changes

octogonz added 10 commits September 19, 2025 00:37

In a Rush repo, the ".pnpmfile.cjs" tab should show Rush's file, not …

6f24527

…the generated temp file

Upgrade Prettier to support async using

2d5b7b8

prettier -w .

ce37f18

PR feedback: use "await using"

2c7c7b3

PR feedback

3fa4702

PR feedback

b86e85d

rush change

e45e089

PR feedback

a387e75

Fix build break

4c95005

Revert [Symbol.asyncDispose]() because it isn't supported by Node 18

fa57502

octogonz merged commit c472ad5 into microsoft:main Sep 21, 2025
5 checks passed

github-project-automation bot moved this from Needs triage to Closed in Bug Triage Sep 21, 2025

octogonz deleted the octogonz/lfx-fixes4 branch September 21, 2025 23:42

Conversation

octogonz commented Sep 17, 2025

Summary

Details

How it was tested

Impacted documentation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

octogonz Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

octogonz Sep 19, 2025 •

edited

Loading